Use Crystal::PointerLinkedList instead of Deque in Mutex
#15330
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The flat array also doesn't have much impact since it's never traversed: we only reach the head or the tail once (to dequeue or dequeue one). This spares a number of GC allocations since the Deque needs to be allocated and also a buffer that will have to be reallocated sometimes (and will only ever grow).
|
This PR seems to remove the need for |
|
Well What we really need is to know whether there's a waiter and it must be atomic, so unlock can safely spare acquiring the spinlock when there are no waiters. Now, we don't need to count: it could be a simple flag on the spinlock value, set when |
straight-shoota
changed the title
Crystal::PointerLinkedList instead of Deque in Mutex
straight-shoota
approved these changes
Jan 14, 2025
straight-shoota
pushed a commit
that referenced
this pull request
Jan 20, 2025
Changes the `flag` that keeps track of initialization status of constants and class variables to have 3 states instead of 2. The third one indicates that the value is currently being initialized and allows detecting recursion. Previously we used an array to keep track of values being currently initialized. This array is now unnecessary. The signature of `__crystal_once` changes: it now takes an Int8 (i8) instead of Bool (i1) and drops the once `state` pointer which isn't needed anymore. So `__crystal_once_init` no longer initializes a state pointer and returns nil. Also introduces a fast path for the (very likely) scenario that the variable is already initialized which doesn't need a mutex. Also introduces an LLVM optimization that instructs LLVM to optimize away repeated calls to `__crystal_once` for the same initializer. Requires a new compiler build to benefit from the improvement. The legacy versions of `__crystal_once` and `__crystal_once_init` are still supported by both the stdlib and the compiler to keep both forward & backward compatibility (1.15 and below can build 1.16+ and 1.16+ can build 1.15 and below). A follow-up could leverage `ReferenceStorage` and `.unsafe_construct` to inline the `Mutex` instead of allocating in the GC heap. Along with #15330 then `__crystal_once_init` could become allocation free, which could prove useful for such a core/low level feature. Co-authored-by: David Keller <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follows
WaitGroupthat uses a linked list instead of a deque to store the waiting fibers.The flat array doesn't have much impact on performance: we only reach the head or the tail once to dequeue/dequeue one fiber at a time. This however spares a number of GC allocations since the Deque has to be allocated plus its buffer that will have to be reallocated sometimes (and will only ever grow, never shrink).
Extracts the undocumented
Fiber::Waitingstruct fromWaitGroupthat acts as the node in the linked list.Renames
@lockto@spinsince the former was a bit confusing, for example with@lock_countthat counts the number of reentrancy.